Remove windows-sys dependency
#374
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jake-Shadle
commented
Mar 20, 2023
Comment on lines
-63
to
65
| Box::from_raw(backend_ptr); | ||
| let _ = Box::from_raw(backend_ptr); | ||
| &*global_backend_ptr |
Comment on lines
-166
to
167
| // Note that this is manually defined here rather than using the definition | ||
| // through `winapi`. The `winapi` definition comes from the `synchapi` | ||
| // header which enables the "synchronization.lib" library. It turns out, | ||
| // however that `Sleep` comes from `kernel32.dll` so this activation isn't | ||
| // necessary. | ||
| // | ||
| // This was originally identified in rust-lang/rust where on MinGW the | ||
| // libsynchronization.a library pulls in a dependency on a newer DLL not | ||
| // present in older versions of Windows. (see rust-lang/rust#49438) | ||
| // | ||
| // This is a bit of a hack for now and ideally we'd fix MinGW's own import | ||
| // libraries, but that'll probably take a lot longer than patching this here | ||
| // and avoiding the `synchapi` feature entirely. | ||
| extern "system" { | ||
| fn Sleep(a: u32); | ||
| } | ||
| unsafe { |
There was a problem hiding this comment.
This comment was referring to the previous winapi crate usage, and was misleading and irrelevant.
Comment on lines
-23
to
19
| WaitOnAddress: extern "system" fn( | ||
| Address: *mut ffi::c_void, | ||
| CompareAddress: *mut ffi::c_void, | ||
| AddressSize: usize, | ||
| dwMilliseconds: u32, | ||
| ) -> BOOL, | ||
| WakeByAddressSingle: extern "system" fn(Address: *mut ffi::c_void), | ||
| WaitOnAddress: WaitOnAddress, | ||
| WakeByAddressSingle: WakeByAddressSingle, | ||
| } |
There was a problem hiding this comment.
These functions were incorrectly not marked unsafe.
Comment on lines
-35
to
24
| // MSDN claims that that WaitOnAddress and WakeByAddressSingle are | ||
| // located in kernel32.dll, but they are lying... | ||
| let synch_dll = unsafe { GetModuleHandleA(b"api-ms-win-core-synch-l1-2-0.dll\0".as_ptr()) }; |
There was a problem hiding this comment.
This comment is outdated, MSDN does show this as the library that implements those functions.
26 tasks
Amanieu
reviewed
Mar 20, 2023
faern
reviewed
Mar 20, 2023
|
Reminder to adopt standalone code generation support as this PR alone leaves you vulnerable to linker issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This replaces the
windows-sysdependency with manual bindings to only the functions, types, and constants needed byparking_lot.Rationale
parking_lotis a foundational dependency, and thus its choice of dependencies has a fairly wide effect on the ecosystem since it's almost inevitable that even a moderately sized dependency graph will pull it in. This is particularly true due the problematic issue I pointed out in microsoft/windows-rs#1720, in which I actually used parking_lot as an example due to its foundational nature. Using bindings in the few places they are needed means we can completely get rid of thewindows-sysdependency, positively impacting a huge part of the crates ecosystem with no negatives.parking_lot_corealready has manual bindings toNt*functions since they are not officially supported by Microsoft and thus not part of the Win32 metadata used to generate the windows-sys bindings.See #332 for a related open issue, and #334 (comment)